-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: improve code index error handling and add text-embedding-v4 support #7357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Enhanced error handling in startIndexing to show detailed validation errors - Added support for text-embedding-v4 model in OpenAI Compatible provider - Added user-friendly error messages with proper translations - Fixes #7356
|
|
||
| // Show error notification to user | ||
| vscode.window.showErrorMessage( | ||
| t("embeddings:validation.initializationFailed", { error: errorMessage }) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using inline fallback English strings in translation calls. The t() calls (e.g. for 'initializationFailed' and 'startIndexingFailed') already have definitions in the i18n JSON; remove the || fallback portions.
| t("embeddings:validation.initializationFailed", { error: errorMessage }) || | |
| t("embeddings:validation.initializationFailed", { error: errorMessage }), |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code because apparently I trust no one, not even myself.
| `Code indexing initialization failed: ${errorMessage}`, | ||
| ) | ||
| return | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice there's duplicate error handling logic between the initialization (lines 2269-2294) and re-initialization (lines 2302-2329) blocks. Could we extract this into a helper function to reduce duplication? Something like:
failed: Code indexing initialization failed:
| `Code indexing initialization failed: ${errorMessage}`, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding test coverage for these new error handling scenarios. Would be good to have tests for:
- Initialization failure with text-embedding-v4
- Error message display to users
- Re-initialization failure scenarios
This would help ensure the error handling continues to work as expected.
| manager.startIndexing() | ||
|
|
||
| // If startIndexing recovered from error, we need to reinitialize | ||
| if (!manager.isInitialized) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is immediate re-initialization after failure intentional? Consider adding a delay or maximum retry count to prevent rapid repeated failures that could impact performance.
| "text-embedding-3-small": { dimension: 1536, scoreThreshold: 0.4 }, | ||
| "text-embedding-3-large": { dimension: 3072, scoreThreshold: 0.4 }, | ||
| "text-embedding-ada-002": { dimension: 1536, scoreThreshold: 0.4 }, | ||
| "text-embedding-v4": { dimension: 1536, scoreThreshold: 0.4 }, // Added support for text-embedding-v4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment could be more descriptive. Consider:
|
This is not a proper fix |
Summary
This PR fixes the Code Index error reported in #7356 that occurs when clicking the Start Indexing button with OpenAI Compatible API provider using the text-embedding-v4 model.
Changes
1. Added Support for text-embedding-v4 Model
2. Enhanced Error Handling
3. User-Friendly Error Messages
Testing
Fixes
Fixes #7356
Screenshots
The error is now properly displayed to users with actionable information instead of failing silently.
Checklist
Important
Fixes Code Index error and adds
text-embedding-v4support with improved error handling and user feedback.text-embedding-v4model inwebviewMessageHandler.text-embedding-v4to supported models inembeddingModels.tswith dimension 1536 and score threshold 0.4.startIndexingto catch initialization errors and display detailed messages.embeddings.json.embeddings.json.This description was created by
for f31560a. You can customize this summary. It will automatically update as commits are pushed.